Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix attachment detection #197

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix attachment detection #197

wants to merge 2 commits into from

Conversation

Roobiz
Copy link

@Roobiz Roobiz commented May 3, 2023 •
edited
Loading

Hi there,

Description

Attachment detection on specific case do not works as expected (inline attachment with encoded filename)

How to reproduce

Tested on latest dovecot version 2.3.20 with the following option:

mail_attachment_detection_options = add-flags-on-save

  1. Simply add the following email to a mailbox sample.eml.txt
  2. Check out the email flags. The email is flagged with $HasNoAttachment instead of $HasAttachment

Description of the issue

In some case mime-part could have encoded header parameters. It was originally define in the RFC 5987
https://datatracker.ietf.org/doc/html/rfc5987 for the http headers but some applications use it in mime context.

For example Mail.app, adding an inline attachment named "Capture d’écran 2023-05-03 à 11.11.51.png" will produce this kind of part headers:

--Apple-Mail=_7891A530-DCCC-4BEA-8EEA-00884364DDF8
Content-Transfer-Encoding: base64
Content-Disposition: inline;
	filename*=utf-8''Capture%20d%E2%80%99e%CC%81cran%202023%2D05%2D03%20a%CC%80%2011.11.51.png
Content-Type: image/png;
	x-mac-hide-extension=yes;
	x-unix-mode=0644;
	name="=?utf-8?B?Q2FwdHVyZSBk4oCZZcyBY3JhbiAyMDIzLTA1LTAzIGHMgCAxMS4xMS41MS5w?=
 =?utf-8?B?bmc=?="
Content-Id: <2EFC0CEF-22B3-44F5-B767-88063E274334>

Attachment detection for inline part defined in the lib-mail/message-part-data was strict on the header parameter and only
check for the parameter==="filename".

The fix

I've simply add another condition on the inline attachment detection to handle encoded "filename"

@sirainen
Copy link
Contributor

sirainen commented May 3, 2023

Hmm. This seems to be RFC 5987 encoding, but that's intended for HTTP only. I wonder how common this is in emails? There is also RFC 2231, which should use filename*0 for detection.

@Roobiz
Copy link
Author

Roobiz commented May 3, 2023 •
edited
Loading

Hmm. This seems to be RFC 5987 encoding, but that's intended for HTTP only. I wonder how common this is in emails? There is also RFC 2231, which should use filename*0 for detection.

Yep, this what I've describe but unfortunately some apps use this syntax, at least mail.app on MacOS, and iPhone Mail on iOS which are a major mail clients.

@sirainen
Copy link
Contributor

sirainen commented May 7, 2023

Yep, this what I've describe but unfortunately some apps use this syntax, at least mail.app on MacOS, and iPhone Mail on iOS which are a major mail clients.

So it seems.. I guess it could be worth it then. Thanks, I'll add this to our internal Jira.

@sirainen
Copy link
Contributor

sirainen commented May 7, 2023

I'm actually wondering now whether we should fully support decoding the parameter in lib-mail, similar to how rfc2231_parse() works now. It converts the filename*0 etc parameters into a single filename. So should we have rfc5987_parse() which does the same thing, so then attachment detection code wouldn't need to be touched?..

@sirainen
Copy link
Contributor

Tracking this internally in DOP-3186

@Roobiz
Copy link
Author

Roobiz commented May 15, 2023

@sirainen it's up to you to choose the better way to manage this issue. Maybe this could also be a slight update un the existing rfc2231 parse method ? For now my proposal, which I know is not perfect, but its the one with the lowest impact on the codebase on my opinion.
Let me know what solution you want and tell me if you want me to investigate and submit new updates on the codebase to handle this :)

@exi
Copy link

exi commented Jun 22, 2024

Any updates on this? I can't search for any mails with attachments because of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Roobiz @sirainen @exi